Document Battle Castle App setup functions#941
Conversation
VicSevenT
force-pushed
the
battle_castle_app_2
branch
from
42a1af2 to
f8c4280
Compare
ravepossum
left a comment
ravepossum
left a comment
There was a problem hiding this comment.
Looks good! I do think Self is more awkward than Player, but I also think it makes sense to match the terminology the game actually uses. I could probably be convinced either way, but Self seems fine to me.
src/overlay104/ov104_0223B6F4.c
Outdated
| @@ -31,7 +31,7 @@ u8 ov104_0223B7DC(u8 param0, BOOL param1); | |||
| FieldBattleDTO *ov104_0223B810(UnkStruct_ov104_0223BA10 *param0, UnkStruct_ov104_02230BE4 *param1); | |||
| static u32 ov104_0223B9E4(u8 param0); | |||
| u8 ov104_0223BA10(UnkStruct_ov104_0223BA10 *param0); | |||
| BOOL ov104_0223BA14(u8 param0); | |||
| BOOL BattleCastle_IsMultiPlayerChallenge(u8 param0); | |||
There was a problem hiding this comment.
polish: We can remove this unnecessary declaration.
include/overlay104/ov104_0223B6F4.h
Outdated
| FieldBattleDTO *ov104_0223B810(UnkStruct_ov104_0223BA10 *param0, UnkStruct_ov104_02230BE4 *param1); | ||
| u8 ov104_0223BA10(UnkStruct_ov104_0223BA10 *param0); | ||
| BOOL ov104_0223BA14(u8 param0); | ||
| BOOL BattleCastle_IsMultiPlayerChallenge(u8 param0); |
There was a problem hiding this comment.
polish: param0 -> challengeType
| } | ||
|
|
||
| return 0; | ||
| return challengeType == 2 || challengeType == 3; |
There was a problem hiding this comment.
question: If we know these indicate multiplayer challenges, can we document the constants at this point?
There was a problem hiding this comment.
Yes, I am planning on documenting these, but I'd like to leave that out of this PR for now. I've been waiting because I wasn't sure what "3" corresponded to ("2" is for multi-battles as initiated from inside the Battle Castle). I now am reasonably sure that "3" is for multi-battle started through the Wi-fi club, but want to handle this separately in a dedicated PR as I think the Battle Hall, and probably all the non-Tower facilities use the same values.
| @@ -9,7 +9,7 @@ | |||
|
|
|||
| #define BASE_TILE_WINDOW_FRAME (1024 - STANDARD_WINDOW_TILE_COUNT) | |||
|
|
|||
| static const WindowTemplate Unk_ov107_0224A288[] = { | |||
| static const WindowTemplate sSelfAppWinTemplates[] = { | |||
There was a problem hiding this comment.
suggestion: Can you format these templates like (for example) sStaticWindowTemplates in src/applications/pokemon_summary_screen/window.c?
Here's a quick and dirty regex I wrote for replacing these if that would help:
Match:
(\s+){\s+(\w+), (\w+), (\w+), (\w+), (\w+), (\w+), (\w+) }
Substitution:
$1{\n$1 .bgLayer = $2,\n$1 .tilemapLeft = $3,\n$1 .tilemapTop = $4,\n$1 .width = $5,\n$1 .height = $6,\n$1 .palette = $7,\n$1 .baseTile = $8,\n$1}
Getting constants for the window IDs like the summary screen windows at some point would be nice as well.
There was a problem hiding this comment.
Reformatted. And I am planning to add window constants in the next PR.
| { | ||
| const WindowTemplate *v1 = sWinTemplates[param2].templates; | ||
| u32 numTemplates = sWinTemplates[param2].numTemplates; | ||
| const WindowTemplate *v1 = sWinTemplates[isOpponentApp].templates; |
There was a problem hiding this comment.
suggestion: template or something for v1 seems appropriate here.
| { | ||
| UnkStruct_ov107_02246170 *v0 = ApplicationManager_Data(appMan); | ||
| BattleCastleOpponentApp *v0 = ApplicationManager_Data(appMan); |
There was a problem hiding this comment.
suggestion: app seems sufficient, like in the Init func and Exit funcs
| } | ||
| break; | ||
| case 4: | ||
| if (ov107_02246D3C(v0) == 1) { | ||
| if (State_FadeOutApp(v0) == 1) { | ||
| return 1; |
There was a problem hiding this comment.
polish: TRUE. Similar for the FALSE in the return statement below.
|
|
||
| param0->unk_3E8 = BattleCastleAppSprite_New(¶m0->unk_1D8, 0, 0, 0, 4, 160, 10, 0, NULL); | ||
| param0->unk_3EC = BattleCastleAppSprite_New(¶m0->unk_1D8, 0, 0, 0, 5, 160, 124, 0, NULL); | ||
| app->upArrowSprite = BattleCastleAppSprite_New(&app->spriteMan, 0, 0, 0, 4, 160, 10, 0, NULL); |
There was a problem hiding this comment.
note: Not blocking for this PR, but it would be good to get constants for the different assets' resource IDs.
There was a problem hiding this comment.
I was also looking at creating constants for the animation IDs in a future PR.
3 participants
Continuing off of #923, this PR focuses mainly on the functions in each app that handle setting up the graphics and a few other initializations. A lot of this is boilerplate that is very similar to the Battle Hall app (#911), not getting into the app's core logic yet.
The biggest item of note here I think is the naming of the two apps as
BattleCastleSelfAppandBattleCastleOpponentApp. I chose these names based off the list items that open each of them when in between rounds: "SELF" and "OPPONENT". The first allows the player to view and modify their own lineup, the latter to view and modify the upcoming opponent.